Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

migrate code base to media-query-list-parser #44

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

migrate code base to media-query-list-parser #44

wants to merge 9 commits into from

Conversation

romainmenke
Copy link

@romainmenke romainmenke commented Nov 14, 2022 •
edited
Loading

see :

summary :

  • travis -> github workflow
  • node 10 -> node 14
  • javascript -> typescript
  • regexp -> media-query-list-parser

Our media query list parser has a very large OM and it is a low level API.
Working with this in plain JavaScript is really painful. It is much easier to work with from TypeScript.

To support TypeScript I've added the build config (rollup) from the csstools mono repo.
There are other ways to get a similar result, but this is one I am comfortable with.

This pull request fixes a few issues but mostly focusses on using the new parser and OM to achieve the same outcome.

note :

Totally fine if this is not a direction you want to take with this plugin.
What works for us doesn't necessarily work for you :)

@romainmenke
Copy link
Author

@yisibl Do you have time in the next few weeks to look at this?

We are hoping to wrap up all work for postcss-preset-env version 8 and would like to include an updated version of this plugin.

As I said before we totally understand it if you want to go in another direction.

@yisibl
Copy link
Member

yisibl commented Nov 30, 2022

Thank you for your contribution, I need to chat with my partner and get back to you.

@romainmenke
Copy link
Author

@yisibl Any news here?

@yisibl
Copy link
Member

yisibl commented Jan 16, 2023

Thank you for your continued interest, I should have time during the Chinese New Year to carry out my work here.

romainmenke
romainmenke commented Jan 16, 2023
View reviewed changes
index.js Show resolved Hide resolved
@romainmenke
Copy link
Author

romainmenke commented Jan 16, 2023 •
edited
Loading

Related issues that will likely be fixed by this change :

@romainmenke romainmenke mentioned this pull request Jan 23, 2023
Open
@romainmenke
Copy link
Author

@yisibl Any chance you could give this another look?
Again totally fine if this parser is not something you want to adopt but we really want to ship an improved version for postcss-preset-env. We also don't mind maintaining a fork but we prefer to contribute here.

The parser and underlying tokenizer have been in use now for months in both postcss-preset-env and stylelint without any issues.

@yisibl
Copy link
Member

yisibl commented Mar 29, 2023

@romainmenke Sorry, too busy with work this month, I'm afraid I'll have time next month!

@romainmenke
Copy link
Author

That is fine @yisibl :)

We really want to move forward with this, given the momentum this feature currently has in browsers.

I've created a new plugin based on this PR in the csstools mono repo.
I've also added a bunch more tests, including browser tests.

Ideally these patches find there way back to this plugin in some form or another after which we can delete the forked plugin and set postcss-preset-env right again :)

I've already updated this PR with the latest bug fixes.
I haven't updated the PR yet that doesn't include TypeScript.

@romainmenke romainmenke mentioned this pull request May 12, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yisibl yisibl yisibl left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@romainmenke @yisibl